Issue455 refresh issue due to pom generated in target on building the project #462
Conversation
| @@ -32,7 +32,7 @@ export const COMMAND_AND_PROJECT_TYPE_MAP: { [command: string]: string[] } = { | |||
| "gradle":[ LIBERTY_GRADLE_PROJECT, LIBERTY_GRADLE_PROJECT_CONTAINER], | |||
| "liberty.dev.debug": [LIBERTY_MAVEN_PROJECT, LIBERTY_GRADLE_PROJECT, LIBERTY_MAVEN_PROJECT_CONTAINER, LIBERTY_GRADLE_PROJECT_CONTAINER], | |||
| }; | |||
| export const EXCLUDED_DIR_PATTERN = "**/{bin,classes,target}/**"; | |||
| export const EXCLUDED_DIR_PATTERN = "**/{bin,classes,target,build}/**"; | |||
There was a problem hiding this comment.
What is EXCLUDED_DIR_PATTERN used for? I see that target was already included, so this must not be working for the refresh scenario. What is the reason for adding build here?
There was a problem hiding this comment.
In refresh method we are calling updateProjects() which will be looking into all the build files in the workspace . there we also mention the directories to exclude for this search and build directory was missing in EXCLUDED_DIR_PATTERN. since we want to avoid to looking into the build folder in case of gradle projects i included 'build' in EXCLUDED_DIR_PATTERN
| // Listens for any new folders are added to the workspace | ||
| context.subscriptions.push(vscode.workspace.onDidChangeWorkspaceFolders((event) => { | ||
| // Listens for any new folders are added to the workspace | ||
| context.subscriptions.push(vscode.workspace.onDidChangeWorkspaceFolders((event) => { |
There was a problem hiding this comment.
Please do not modify spacing or formatting on existing code, unless it is in a separate PR specific to formatting changes. It is not immediately clear if there are any functional changes in the middle of formatting changes, which makes code more difficult to review and makes it more difficult for others to understand when looking back at code history.
| * @returns | ||
| */ | ||
| export function isInTargetOrBuild(filePath: string): boolean { | ||
| return filePath.includes('/target/') || filePath.includes('/build/'); |
There was a problem hiding this comment.
I don't think this is valid if it is just any "target" or "build" dir. We need to ensure the file/path is in a subdirectory of the project's "target" or "build" directory (i.e. the "target" or "build" directory located in the root project folder).
| projectProvider.refresh(); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
I am not entirely sure about this approach, since we are still watching the "target" and "build" dirs. It would be better if we could find a way to ignore them completely.
There was a problem hiding this comment.
can we add '&&' operator in vscode.workspace.createFileSystemWatcher?
Did conditions like this work '**/pom.xml && !inside target folder' ?
There was a problem hiding this comment.
for glob patterns && operators are not available in their syntax, however negations are available as per their documentation
https://code.visualstudio.com/docs/editor/glob-patterns
There was a problem hiding this comment.
@JoshwinThomasIBM {} to group conditions (for example {**/*.pom.xml,!inside target folder} )
Can you try with this group conditions?
There was a problem hiding this comment.
@aparnamichael i tried with "/!(target)//{pom.xml,build.gradle,settings.gradle,src/main/liberty/config/server.xml}
and it seems to be working , but multiple negations is not supported in glob pattern
also with the above glob patten we expect only the one's in target to be negated and other pom's generated to be watched ,but if the the pom is generated in some other location then that is not being caught by the watcher .
Fixes #455